Skip to content

Conversation

@1-leo
Copy link
Contributor

@1-leo 1-leo commented Dec 5, 2025

No description provided.

@1-leo
Copy link
Contributor Author

1-leo commented Dec 5, 2025

  • signers now return a event because its immutable
  • => problem in NdkBroadcastResponse because publishEvent need to be a future TBD
  • check AuthEvent extend
  • check ZapRequest extend

@codecov
Copy link

codecov bot commented Dec 6, 2025

Codecov Report

❌ Patch coverage is 88.30189% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.22%. Comparing base (f368849) to head (22f7e13).
⚠️ Report is 29 commits behind head on fix-websocket-performance-issues-2.

Files with missing lines Patch % Lines
...es/ndk/lib/domain_layer/entities/nip_01_utils.dart 73.68% 10 Missing ⚠️
...kages/ndk/lib/shared/isolates/isolate_manager.dart 65.38% 9 Missing ⚠️
...in_layer/usecases/proof_of_work/proof_of_work.dart 60.00% 4 Missing ⚠️
...layer/repositories/signers/nip46_event_signer.dart 0.00% 2 Missing ⚠️
...ndk/lib/domain_layer/usecases/bunkers/bunkers.dart 0.00% 2 Missing ⚠️
.../ndk/lib/data_layer/models/nip_01_event_model.dart 98.38% 1 Missing ⚠️
...ayer/repositories/signers/bip340_event_signer.dart 50.00% 1 Missing ⚠️
...s/ndk/lib/domain_layer/usecases/relay_manager.dart 93.75% 1 Missing ⚠️
packages/ndk/lib/shared/nips/nip13/nip13.dart 93.33% 1 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                           @@
##           fix-websocket-performance-issues-2     #332      +/-   ##
======================================================================
+ Coverage                               70.87%   71.22%   +0.35%     
======================================================================
  Files                                     139      142       +3     
  Lines                                    5157     5262     +105     
======================================================================
+ Hits                                     3655     3748      +93     
- Misses                                   1502     1514      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@1-leo 1-leo requested review from frnandu and nogringo December 6, 2025 10:34
@1-leo
Copy link
Contributor Author

1-leo commented Dec 6, 2025

@frnandu
We have this in both our engines with the refactor, the not signed event is returned.
I think the best way forward is to return a future (with a completer), but this would require more refactoring on all the clients...
What do you think?

grafik grafik

@1-leo
Copy link
Contributor Author

1-leo commented Dec 6, 2025

  • refactor nip13 to non static (object model)
  • isolate without Singleton? As usecase

@1-leo 1-leo self-assigned this Dec 6, 2025
@1-leo 1-leo added this to ndk-dev Dec 6, 2025
@1-leo 1-leo moved this to In Progress in ndk-dev Dec 6, 2025
@1-leo 1-leo added the refactor label Dec 6, 2025
@frnandu frnandu marked this pull request as ready for review December 17, 2025 13:17
@frnandu
Copy link
Collaborator

frnandu commented Dec 22, 2025

My proposed changes (to be discussed):

  • re-naming Nip01EventService -> Nip01Utils and not in the usecases package
  • dart:isolate is not supported on dart4web -> we need to either try/catch or use stubs depending on dart.library.html???
DartError: Unsupported operation: dart:isolate is not supported on dart4web
dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 274:3       throw_
dart-sdk/lib/_internal/js_dev_runtime/patch/isolate_patch.dart 149:3              _unsupported
dart-sdk/lib/_internal/js_dev_runtime/patch/isolate_patch.dart 117:19             get sendPort
package:ndk/shared/isolates/isolate_manager.dart 51:67                            <fn>
  • in Nip01Event we can use late final String id; and then this.id = id ?? Nip01Utils.calculateEventIdSync in the constructor and still keep it immutable. No need for such big refactoring and break existing client app's code.

  • why we need an isolate manager for json decoding, can't we also use Isolate.run for only such simple things as json decode? Or maybe try to use a ready lib like isolate_manager or work_manager instead of trying to do it ourselves?

see #348

Copy link
Collaborator

@frnandu frnandu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see previous comment

@1-leo
Copy link
Contributor Author

1-leo commented Dec 22, 2025

#332 (comment)
=>
Todo

  • fix web
  • relay pool, IsolateManager

@frnandu frnandu added this to the 0.7 milestone Dec 22, 2025
@1-leo
Copy link
Contributor Author

1-leo commented Dec 24, 2025

noticed some weird behavior, if encodingIsolatePoolSize > 1 the tests fail. Looks like our code depends on the order of processed events, suggesting thats its not safe for concurrency. (see tests above)
@frnandu did you have the same problem when using Isolate.spawn() directly?

@1-leo
Copy link
Contributor Author

1-leo commented Dec 24, 2025

I think the problem is that processing the eose event is always faster compared to a kind 1 etc.
Rn I dont see another option then to use debounce or to wait until all events are processed

@1-leo
Copy link
Contributor Author

1-leo commented Dec 24, 2025

@frnandu please take a close look at 22f7e13 I am not sure if its the best approach to use a chain here. We may only care about the eose to be in order

@1-leo 1-leo requested a review from frnandu January 12, 2026 17:47
@1-leo
Copy link
Contributor Author

1-leo commented Jan 12, 2026

@frnandu are the issues now resolved?
The isolates pool is also in this pr

@frnandu
Copy link
Collaborator

frnandu commented Jan 12, 2026

@frnandu are the issues now resolved?
The isolates pool is also in this pr

I'll test/review it tomorrow

@1-leo 1-leo mentioned this pull request Jan 13, 2026
@1-leo 1-leo merged commit 190303d into fix-websocket-performance-issues-2 Jan 14, 2026
3 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in ndk-dev Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants